PR Review Report

Overall Assessment

⚠️ Needs changes: one unmitigated P1 finding from the required sibling-surface mirror audit.

Executive Summary

• Affected method: ChaisemartinDHaultfoeuille dCDH reversible-treatment heterogeneity test, Web Appendix Section 1.5 / Lemma 7, under by_path and paths_of_interest.
• Methodology registry now documents the per-path heterogeneity contract and the R-parity scope; I did not find an undocumented math or SE deviation in the estimator path.
• Inference uses safe_inference() on the new path; pattern-wide grep found no new inline t_stat = effect / se anti-pattern.
• P1: the new path_heterogeneity_effects surface is wired into the result dataclass and to_dataframe(level="by_path"), but not into summary(), unlike sibling per-path surfaces and global heterogeneity.
• Could not run tests: pytest is not installed in this environment.

Methodology

Finding M-1: P3 Informational — documented partial/deviation surface, no action required

Severity: P3
Impact: The global heterogeneity implementation remains a documented partial predict_het port: post-treatment regressions only, no placebo regressions or joint null test. This is explicitly documented in docs/methodology/REGISTRY.md:L637, so it is not a defect under the review rules. The new per-path extension is also documented in the by-path note, including path-restricted switchers, unchanged variance machinery, survey composition, final df_survey refresh, and result schema at docs/methodology/REGISTRY.md:L643.
Concrete fix: None required.

Code Quality

Finding CQ-1: P1 — summary() does not render the new per-path heterogeneity surface

Severity: P1
Impact: The PR adds results.path_heterogeneity_effects and exposes it in to_dataframe(level="by_path"), but the sibling report surface is missing. Global heterogeneity has a dedicated summary renderer at diff_diff/chaisemartin_dhaultfoeuille_results.py:L1253-L1283, and existing per-path sibling surfaces render placebos, cumulated effects, and sup-t bands inside _render_path_effects_section at diff_diff/chaisemartin_dhaultfoeuille_results.py:L1308-L1431. The new heterogeneity field is only consumed by the DataFrame path at diff_diff/chaisemartin_dhaultfoeuille_results.py:L1823-L1906. This violates the required sibling-surface mirror audit (schema ↔ renderer, summary ↔ full) and makes summary() silently omit a computed public result.
Concrete fix: Extend _render_path_effects_section() to read self.path_heterogeneity_effects.get(path, {}) and render a per-path heterogeneity sub-block with beta, se, t_stat, and p_value, mirroring _render_heterogeneity_section(). Add a regression test asserting res.summary() contains the per-path heterogeneity block and path/horizon rows when by_path + heterogeneity is fit.

Performance

No P0/P1/P2 findings.

Maintainability

Finding MA-1: P1 — same as CQ-1

Severity: P1
Impact: The result schema and renderer are now out of sync for one public surface, increasing the chance future users or downstream tests treat summary() as complete when it is not.
Concrete fix: Same as CQ-1.

Tech Debt

No mitigating TODO entry found under TODO.md “Tech Debt from Code Reviews” for the missing summary surface. The P1 remains unmitigated.

Security

No findings.

Documentation/Tests

Finding DT-1: P2 — summary coverage missing for the new public surface

Severity: P2
Impact: The new tests cover gate dispatch, behavior, DataFrame integration, and R parity, but do not cover summary() for path_heterogeneity_effects. Existing sibling features have summary tests, so this gap allowed CQ-1 through.
Concrete fix: Add a TestByPathHeterogeneity::test_per_path_heterogeneity_renders_in_summary test.

Verification note: I could not run the targeted tests because pytest is not installed (/bin/bash: pytest: command not found). I did run static greps for the inference anti-pattern and sibling surfaces.

Path to Approval

1. Add summary rendering for path_heterogeneity_effects inside _render_path_effects_section().
2. Add a regression test that fits by_path + heterogeneity and asserts summary() includes the per-path heterogeneity rows.
3. Run the new test plus the existing TestByPathHeterogeneity and dCDH parity heterogeneity tests in an environment with pytest installed.

/ai-review

🔁 AI review rerun (requested by @igerber)

Head SHA: 7f7e3d5bec9fb80aadcb80f66f0fb4a408514077

Overall Assessment

✅ Looks good — no unmitigated P0 or P1 findings in this re-review.

Executive Summary

• Prior P1 is resolved: summary() now renders per-path heterogeneity rows at diff_diff/chaisemartin_dhaultfoeuille_results.py:L1421-L1443, with a regression test at tests/test_chaisemartin_dhaultfoeuille.py:L10331-L10370.
• Affected method: dCDH reversible-treatment heterogeneity test, Web Appendix Section 1.5 / Lemma 7, under by_path / paths_of_interest.
• Methodology is documented in docs/methodology/REGISTRY.md:L637 and docs/methodology/REGISTRY.md:L643; I found no undocumented estimator, weighting, or SE deviation.
• New inference paths use safe_inference() and preserve NaN-consistent inference fields.
• Completed sibling-surface, pattern-wide grep, and by_path ↔ paths_of_interest symmetry audits. No workflow paths: changes were in scope.
• I could not run tests locally because pytest is unavailable in this environment.

Methodology

M-1: P3 Informational — documented partial R predict_het surface

Severity: P3
Impact: The global heterogeneity implementation remains a documented partial port: post-treatment regressions only, without R’s placebo regressions or joint null test. This is explicitly documented as a deviation in docs/methodology/REGISTRY.md:L637, so it is not a defect under the review rules. The new per-path extension is also documented, including path-restricted switchers, variance machinery, survey composition, df refresh, and result schema at docs/methodology/REGISTRY.md:L643.
Concrete fix: None required.

No P0/P1 methodology findings. The new per-path helper restricts eligibility through path_groups inside _compute_heterogeneity_test at diff_diff/chaisemartin_dhaultfoeuille.py:L5025-L5027, and dispatches per selected path at diff_diff/chaisemartin_dhaultfoeuille.py:L6459-L6535.

Code Quality

No P0/P1/P2 findings.

The prior missing renderer is fixed in the sibling report surface at diff_diff/chaisemartin_dhaultfoeuille_results.py:L1421-L1443. Pattern-wide grep found no new inline t_stat = effect / se inference anti-pattern in the changed dCDH path; the new and refreshed fields route through safe_inference() at diff_diff/chaisemartin_dhaultfoeuille.py:L5106-L5223 and diff_diff/chaisemartin_dhaultfoeuille.py:L4058-L4070.

Performance

No findings.

Maintainability

No P0/P1/P2 findings.

Sibling surfaces are now aligned: result schema at diff_diff/chaisemartin_dhaultfoeuille_results.py:L597-L607, summary rendering at diff_diff/chaisemartin_dhaultfoeuille_results.py:L1421-L1443, and to_dataframe(level="by_path") het_* columns at diff_diff/chaisemartin_dhaultfoeuille_results.py:L1781-L1932.

Tech Debt

No untracked blocking tech debt found for this PR. Existing TODO.md review debt entries appear unrelated to the new per-path heterogeneity surface.

Security

No findings. A lightweight secret-pattern scan over the changed text did not find candidate secrets.

Documentation/Tests

DT-1: P3 Informational — local test execution unavailable

Severity: P3
Impact: I could not execute the targeted tests because this environment lacks pytest (/usr/bin/python: No module named pytest). Static review shows coverage was added for gates, behavior, DataFrame output, summary rendering, and R parity at tests/test_chaisemartin_dhaultfoeuille.py:L9977-L10370 and tests/test_chaisemartin_dhaultfoeuille_parity.py:L1324-L1472.
Concrete fix: Run the targeted dCDH heterogeneity and parity tests in CI or a full dev environment with test dependencies installed.

/ai-review

🔁 AI review rerun (requested by @igerber)

Head SHA: 6c8a68ca40f85d2b2cbc6155e5ad0c9e5149de0f

Overall Assessment

✅ Looks good — no unmitigated P0/P1 findings in this re-review.

Executive Summary

• Prior P1 is resolved: summary() now renders per-path heterogeneity rows at diff_diff/chaisemartin_dhaultfoeuille_results.py:L1421-L1443, with coverage at tests/test_chaisemartin_dhaultfoeuille.py:L10534-L10572.
• Affected method: dCDH reversible-treatment heterogeneity test, Web Appendix Section 1.5 / Lemma 7, now under by_path and paths_of_interest.
• Methodology is documented in docs/methodology/REGISTRY.md:L637 and docs/methodology/REGISTRY.md:L643; I found no undocumented estimator, weighting, or SE deviation.
• New inference paths use safe_inference() and refresh replicate-weight df for per-path heterogeneity at diff_diff/chaisemartin_dhaultfoeuille.py:L4058-L4070.
• Completed sibling-surface, pattern-wide grep, by_path ↔ paths_of_interest symmetry, and workflow-scope audits. No GH Actions workflow paths: changes are in scope.
• I could not run tests locally because this environment lacks numpy, pandas, and pytest.

Methodology

M-1: P3 Informational — documented partial R predict_het surface

Severity: P3
Impact: The global heterogeneity implementation remains a documented partial port: post-treatment regressions only, without R’s placebo regressions or joint null test. This is explicitly documented in docs/methodology/REGISTRY.md:L637, so it is not a defect. The new per-path extension is documented at docs/methodology/REGISTRY.md:L643.
Concrete fix: None required.

No P0/P1 methodology findings. The path restriction is implemented through path_groups in _compute_heterogeneity_test() at diff_diff/chaisemartin_dhaultfoeuille.py:L5025-L5027 and dispatched by _compute_path_heterogeneity_test() at diff_diff/chaisemartin_dhaultfoeuille.py:L6459-L6535.

Code Quality

No P0/P1/P2 findings.

The new result schema is wired into the result dataclass at diff_diff/chaisemartin_dhaultfoeuille_results.py:L597-L607, summary rendering at L1421-L1443, and to_dataframe(level="by_path") at L1819-L1937.

Performance

No findings.

Maintainability

No P0/P1/P2 findings.

Sibling surfaces are aligned: estimator output, summary rendering, DataFrame columns, docs, registry, and parity tests all reference the same {path: {l: ...}} path_heterogeneity_effects contract.

Tech Debt

No untracked blocking tech debt found for this PR. Existing TODO/registry limitations are unrelated or explicitly documented.

Security

No findings. A lightweight secret-pattern scan over the changed files found no candidate secrets.

Documentation/Tests

DT-1: P3 Informational — local test execution unavailable

Severity: P3
Impact: I could not execute the targeted tests because this environment lacks required test dependencies. Static review shows coverage for gates, behavior, summary, DataFrame output, survey composition, and R parity.
Concrete fix: Run the targeted dCDH heterogeneity and parity tests in CI or a full dev environment.

DT-2: P3 Informational — multi-baseline no-warning test is not actually multi-baseline

Severity: P3
Impact: tests/test_chaisemartin_dhaultfoeuille.py:L10276-L10302 claims to cover the multi-baseline no-warning anti-regression, but _by_path_het_data() uses paths that all start from baseline 0. This does not block approval because the behavior is documented and the methodology path is not silently wrong.
Concrete fix: Add a small fixture with switchers on at least two baseline values and assert no multi-baseline warning under by_path + heterogeneity.

/ai-review

🔁 AI review rerun (requested by @igerber)

Head SHA: a57f5834601f5b31e7561a7db1e45094acd3323b

Overall Assessment

⚠️ Needs changes — one unmitigated P2 claim-vs-test mismatch remains.

Executive Summary

• Affected method: ChaisemartinDHaultfoeuille dCDH heterogeneity test (Web Appendix Section 1.5 / Lemma 7), newly composed with by_path / paths_of_interest.
• Methodology implementation is broadly consistent with the visible REGISTRY contract: per-path heterogeneity filters switchers by selected path and reuses the existing Lemma 7 regression / inference path.
• Prior summary-rendering concern appears resolved: summary() now renders per-path heterogeneity rows.
• Public surfaces are mostly wired: result dataclass field, summary(), and to_dataframe(level="by_path") all expose path_heterogeneity_effects / het_*.
• Remaining blocker is a test-claim mismatch: the PR/CHANGELOG/REGISTRY claim multi-baseline per-path heterogeneity behavior is regression-tested, but the visible “multi-baseline” test fixture uses only baseline-0 switcher paths.

Methodology

M-1 — P3 Informational: documented partial heterogeneity implementation

Severity: P3
Impact: The dCDH heterogeneity test remains a partial implementation relative to R / the paper: post-treatment regressions only, no placebo regressions or joint null test. This is explicitly documented in the Methodology Registry under the dCDH heterogeneity notes, so it is not a defect.
Concrete fix: None required.

M-2 — No unmitigated methodology mismatch found in implementation

Severity: N/A
Impact: The new implementation path matches the visible REGISTRY claim for per-path heterogeneity: _compute_path_heterogeneity_test() enumerates selected paths and calls _compute_heterogeneity_test(..., path_groups=...), while _compute_heterogeneity_test() restricts eligible switchers via if path_groups is not None and g not in path_groups: continue (diff_diff/chaisemartin_dhaultfoeuille.py:_compute_heterogeneity_test, _compute_path_heterogeneity_test). This is consistent with the documented “path-restricted switcher subsample” contract.
Concrete fix: None.

Code Quality

No P0/P1/P2 code-quality findings in the visible changed source.

The new field is threaded through:

• Fit result construction: diff_diff/chaisemartin_dhaultfoeuille.py near path_heterogeneity_effects=...
• Result dataclass: diff_diff/chaisemartin_dhaultfoeuille_results.py near path_heterogeneity_effects
• Summary rendering: ChaisemartinDHaultfoeuilleResults._render_path_effects_section
• DataFrame rendering: ChaisemartinDHaultfoeuilleResults.to_dataframe(level="by_path")

Performance

No findings.

The path heterogeneity helper re-enumerates treatment paths after path_effects already did so. This is documented in-code and is likely acceptable given the feature scope; no performance blocker.

Maintainability

No unmitigated maintainability findings.

The schema shape {path: {l: {...}}} is documented in source docstrings and handled separately from path_effects[path]["horizons"], which avoids overloading the existing path-effect schema.

Tech Debt

No untracked blocking tech debt found in the visible context.

Security

No findings in the visible diff. I did not run a secret scanner or shell grep; this is based only on the loaded prompt content.

Documentation/Tests

DT-1 — P2: claimed multi-baseline behavior is not actually tested

Severity: P2
Impact: The PR/CHANGELOG/REGISTRY explicitly claim that per-path heterogeneity on multi-baseline switcher panels does not produce the R-divergence warning used for controls / trends_linear, because cohort dummies absorb baseline. The visible test named test_per_path_heterogeneity_no_multi_baseline_warning does not actually construct a multi-baseline switcher panel: _by_path_het_data() defines paths [(0, 1, 1, 1), (0, 1, 0, 0), (0, 1, 1, 0)], all starting from baseline 0 (tests/test_chaisemartin_dhaultfoeuille.py:_by_path_het_data). As a result, the shipped test does not cover the explicitly claimed multi-baseline behavior.

This is a claim-vs-test mismatch under the single-pass audit rules. No TODO mitigation is visible in the prompt, and shipped-behavior test gaps are P2.

Concrete fix: Add or correct a test with at least two switcher baseline values, e.g.:

• Include baseline-0 joiner paths such as (0, 1, 1, 1).
• Include baseline-1 leaver paths such as (1, 0, 0, 0) or another valid reversible path beginning with 1.
• Fit with ChaisemartinDHaultfoeuille(drop_larger_lower=False, by_path=2) or paths_of_interest=[(0,1,1,1), (1,0,0,0)], L_max=3, heterogeneity="het_x".
• Assert:
  1. res.path_heterogeneity_effects contains both selected paths or the expected observed subset.
  2. No warning matching the controls/trends-linear multi-baseline divergence text is emitted.
  3. At least one per-path heterogeneity entry is finite where the design has sufficient observations.

Path to Approval

1. Add a real multi-baseline per-path heterogeneity test as described in DT-1, using switcher paths with different first elements / baseline treatment values.
2. Ensure the new test filters out unrelated expected warnings, but fails on any warning containing the multi-baseline R-divergence language used by by_path + controls / by_path + trends_linear.
3. Re-run the targeted dCDH heterogeneity tests and parity tests in CI.